Skip to content

fix: avoid char-boundary panic in NBReader::try_read#172

Open
SAY-5 wants to merge 2 commits into
rust-cli:mainfrom
SAY-5:fix-try-read-char-boundary
Open

fix: avoid char-boundary panic in NBReader::try_read#172
SAY-5 wants to merge 2 commits into
rust-cli:mainfrom
SAY-5:fix-try-read-char-boundary

Conversation

@SAY-5
Copy link
Copy Markdown

@SAY-5 SAY-5 commented May 12, 2026

The internal buffer stores raw input bytes as latin-1 chars, so a byte >= 0x80 becomes a two-byte char and drain(..1) could land off a char boundary and panic. Drain the first char by its UTF-8 length instead.

Closes #32

Comment thread src/reader.rs Outdated
@SAY-5 SAY-5 force-pushed the fix-try-read-char-boundary branch from 298398d to 534e491 Compare May 13, 2026 00:13
@SAY-5
Copy link
Copy Markdown
Author

SAY-5 commented May 13, 2026

Split into a test commit that shows the panic on multi-byte input followed by the fix, per the contrib guide.

@SAY-5 SAY-5 force-pushed the fix-try-read-char-boundary branch from 534e491 to f9e11ce Compare May 13, 2026 06:41
@SAY-5
Copy link
Copy Markdown
Author

SAY-5 commented May 13, 2026

Restructured per the c-test guide: the test commit now documents the panic with should_panic, and the fix commit drops should_panic and asserts the decoded chars. Pushed.

Comment thread src/reader.rs
// pump bytes from the reader thread into the buffer
for _ in 0..10 {
let _ = r.try_read();
thread::sleep(time::Duration::from_millis(5));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the sleep?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From #172 (comment)

NBReader::read_into_buffer reads from a background thread that pipes the cursor bytes into self.buffer over an mpsc channel, so a single call right after construction usually finds the buffer still empty. The 5ms sleeps give that thread time to deliver bytes between try_read calls. Happy to switch to a synchronous fixture or a poll loop if you'd prefer not to depend on a timed delay.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would prefer to avoid the sleeps. We slow down test runs and risk race conditions.

@SAY-5

This comment was marked as duplicate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

try_read panic

2 participants